fix(openid-connect): token introspection with client_secret_basic sends credentials in both header and body#13524
Conversation
…d body during introspection
|
@kevinlzw Thanks! I verified your fix against APISIX: I ran this PR's test suite (a strict mock introspection endpoint that rejects requests carrying credentials in both the Authorization header and the body) on current master with lua-resty-openidc bumped to 1.9.0 and without the plugin-side workaround from this PR — all tests pass. With 1.8.0 the same tests fail, so the upstream fix alone resolves the problem. One detail worth noting: your fix keeps the legacy body-credentials behavior when Since v1.9.0 is already released and published to LuaRocks, the right fix for APISIX is now simply bumping the dependency in the rockspec instead of patching around it, so I'm closing this PR. Would you like to submit the version-bump PR? Since you authored the upstream fix, it seems fitting — and feel free to pick up |
Description
When
introspection_endpoint_auth_methodisclient_secret_basic(the default), the token introspection request sends the client credentials twice: once in theAuthorization: Basicheader and once more asclient_id/client_secretin the POST body. RFC 6749 Section 2.3.1 says the client must not use more than one authentication mechanism per request, so strict authorization servers reject the introspection call withinvalid_clientand every bearer-token request through the route fails with 401 — with the default plugin config.The root cause is in lua-resty-openidc 1.8.0:
introspect()unconditionally copiesclient_id/client_secretinto the request body, andcall_token_endpoint()then adds the Basic header forclient_secret_basicwithout removing the body copies. There is an upstream issue (zmartzone/lua-resty-openidc#556), but it has been open for a while with no release containing a fix, so this patches it on the APISIX side without forking the library.The fix extends the existing
http_request_decoratorhook usage in the plugin's introspection path: when the auth method isclient_secret_basic, the decorator stripsclient_id/client_secretfrom the urlencoded request body before the request is sent. Scoping notes:openidc.introspect()and cleared right after it returns (existing behavior), so it only affects the introspection call, not the token-endpoint calls of the authorization code flow.openidc.introspect(), the decorator can also see a body-less discovery GET (whenintrospection_endpointis not set explicitly), hence thereq.bodyguard.client_secret_post,private_key_jwtandclient_secret_jwtare untouched: stripping only happens forclient_secret_basic, where the credentials are already carried by the Basic header.The new test file
t/plugin/openid-connect11.tuses a mock introspection endpoint that behaves like a strict authorization server (rejects requests carrying credentials in both the header and the body) and coversclient_secret_basic(default and explicit, with and withoutintrospection_addon_headers) as well asclient_secret_post, which must keep sending body credentials.Which issue(s) this PR fixes:
Fixes #13085
Checklist